-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ruff
NPY002
#162
Fix ruff
NPY002
#162
Conversation
data_dict = { | ||
elem.symbol: np.random.randn(100) + np.random.randn(100) for elem in Element | ||
} | ||
data_dict = {elem.symbol: np_rng.randn(100) + np_rng.randn(100) for elem in Element} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@janosh It's so interesting, as I started working on fixing NPY002
violations in pymatgen
materialsproject/pymatgen#3892 without looking at this PR (really!) almost at the same time.
But this legacy to new Generator replacement is not just replacing np.random
with rng
(the API is not exactly the same, please refer to https://numpy.org/doc/stable/reference/random/generator.html), this should be np_rng.standard_normal(100)
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! thanks for letting me know. i should really take the time to make these scripts run in CI to avoid such mistakes...
will submit a fix now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem at all :) Yes we need to include them into CI tests, mind letting me (learn and) do this?
@@ -8,15 +8,16 @@ | |||
|
|||
# %% Sankey diagram of random integers | |||
cols = ["col_a", "col_b"] | |||
df_rand_ints = pd.DataFrame(np.random.randint(1, 6, size=(100, 2)), columns=cols) | |||
np_rng = np.random.default_rng(seed=0) | |||
df_rand_ints = pd.DataFrame(np_rng.randint(1, 6, size=(100, 2)), columns=cols) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be np_rng.integers()
.
@@ -27,7 +28,7 @@ | |||
ax = error_decay_with_uncert(y_true, y_pred, y_std) | |||
save_and_compress_svg(ax, "error-decay-with-uncert") | |||
|
|||
eps = 0.2 * np.random.randn(*y_std.shape) | |||
eps = 0.2 * np_rng.randn(*y_std.shape) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, should be rng.standard_normal()
y_pred = xs + 0.1 * np.random.normal(size=100) | ||
y_true = xs + 0.1 * np.random.normal(size=100) | ||
np_rng = np.random.default_rng(seed=0) | ||
xs = np_rng.uniform(0, 1, 100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is the same as np_rng.random(100)
?
* apply corrections pointed out by @DanielYang59 in #162 (comment) * explain purpose and inner workings of density_scatter_plotly in doc string * fix docs build * refactor: fig.update_layout(title=dict()) -> fig.layout.title = dict() * more refactor * fix ruff N806
No description provided.